You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Reload context under the stack lock when loading by intent - #538
_load_context_by_intent reads the context before it takes the stack lock. With two quick clicks on one window, both updates read the same version. They then pass through the lock one at a time, and the second saves its stale copy over the first one's result, so a dialog_data change is lost.
The context is now read again once the lock is held. The first read is still needed, because it provides the stack_id used to take the lock.
The reload comes after _check_outdated, which differs from the order suggested in the issue. If the first update closes the dialog, its context is deleted. Reloading first would raise UnknownIntent where this path raises OutdatedIntent today.
This adds one extra load_context storage read for each callback handled by intent.
Tests
Two tests added to tests/test_isolation.py:
test_concurrent_clicks_keep_dialog_data: two concurrent clicks each increment a counter. Without the fix it ends at count=1, with the fix count=2.
test_concurrent_click_after_done_is_outdated: the first click calls done() and the second gets OutdatedIntent. It passes with and without the fix, and fails if the reload is moved before _check_outdated.
ruff check, pytest (84 passed) and vulture are clean on Python 3.12, also from a fresh clone of this branch.
Summary by CodeRabbit
Bug Fixes
Improved intent context handling during validation to ensure the latest context is used.
Concurrent dialog actions now preserve both increments and report outdated intents when applicable.
No actionable comments were generated in the recent review.
Walkthrough
The middleware reloads an intent context after it checks the intent against the stack. Isolation tests cover concurrent clicks that increment dialog data and concurrent clicks that finish the dialog.
Changes: Intent context isolation
Layer / File(s)
Summary
Context reload and concurrent-click coverage src/aiogram_dialog/context/intent_middleware.py, tests/test_isolation.py
The middleware reloads the intent context after the stack check and before access validation. Tests cover two concurrent increment clicks and two concurrent completion clicks, which expect an OutdatedIntent for the outdated click.
The change addresses concurrent dialog updates. No actionable merge-blocking issue was established.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: reloading the context while holding the stack lock during intent loading.
Linked Issues check
Issue #537 requires a second context read after the stack lock is acquired. In IntentMiddlewareFactory._load_context_by_intent, the initial load_context obtains stack_id, _load_stack acquires …
Out of Scope Changes check
The changes are limited to the intent middleware fix for issue #537 and focused tests in tests/test_isolation.py. The test helpers and assertions directly validate concurrent callback isolation and …
Full details: Linked Issues check
Explanation
Issue #537 requires a second context read after the stack lock is acquired. In IntentMiddlewareFactory._load_context_by_intent, the initial load_context obtains stack_id, _load_stack acquires the lock, and a second load_context runs before the context is used and saved. _check_outdated remains before the reload, which preserves OutdatedIntent for a callback whose dialog was closed. The added isolation tests cover concurrent dialog_data increments and the outdated-intent case.
Full details: Out of Scope Changes check
Explanation
The changes are limited to the intent middleware fix for issue #537 and focused tests in tests/test_isolation.py. The test helpers and assertions directly validate concurrent callback isolation and OutdatedIntent behavior. No unrelated production or test changes are shown.
Full details: Docstring Coverage
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 8973b2d3-d919-44d2-9ea8-469929f52ce5
Commits
Reviewing files that changed from the base of the PR and between b6dfeff and 9b72cca.
Files selected for processing: 2
src/aiogram_dialog/context/intent_middleware.py
tests/test_isolation.py
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #537
_load_context_by_intentreads the context before it takes the stack lock. With two quick clicks on one window, both updates read the same version. They then pass through the lock one at a time, and the second saves its stale copy over the first one's result, so adialog_datachange is lost.The context is now read again once the lock is held. The first read is still needed, because it provides the
stack_idused to take the lock.The reload comes after
_check_outdated, which differs from the order suggested in the issue. If the first update closes the dialog, its context is deleted. Reloading first would raiseUnknownIntentwhere this path raisesOutdatedIntenttoday.This adds one extra
load_contextstorage read for each callback handled by intent.Tests
Two tests added to
tests/test_isolation.py:test_concurrent_clicks_keep_dialog_data: two concurrent clicks each increment a counter. Without the fix it ends atcount=1, with the fixcount=2.test_concurrent_click_after_done_is_outdated: the first click callsdone()and the second getsOutdatedIntent. It passes with and without the fix, and fails if the reload is moved before_check_outdated.ruff check,pytest(84 passed) andvultureare clean on Python 3.12, also from a fresh clone of this branch.Summary by CodeRabbit